Backport PR #2577 to release/v1.7 for refactor docker and change buildkit-syft-scanner reference to ghcr.io #2578
Conversation
WalkthroughWalkthroughThe recent changes focus on optimizing and streamlining configuration files across the project, including Dockerfiles, Kubernetes manifests, and GitHub workflows. Key improvements involve the removal of redundant entries, updates to maintainer labels, and enhancements in version control for dependencies. These modifications aim to enhance maintainability, reduce complexity, and ensure improved build and deployment processes across the board. Changes
Sequence Diagram(s)sequenceDiagram
participant User
participant CI/CD
participant Docker
participant Kubernetes
User->>CI/CD: Push code
CI/CD->>Docker: Build image
Docker-->>CI/CD: Image built
CI/CD->>Kubernetes: Deploy new image
Kubernetes-->>CI/CD: Deployment successful
CI/CD-->>User: Notify success
Thank you for using CodeRabbit. We offer it for free to the OSS community and would appreciate your support in helping us grow. If you find it useful, would you consider giving us a shout-out on your favorite social media? TipsChatThere are 3 ways to chat with CodeRabbit:
Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (invoked as PR comments)
Additionally, you can add CodeRabbit Configuration File (
|
kpango
force-pushed
the
backport/main/release/v1.7
branch
from
70e0ab9 to
3845156
Compare
Deploying vald with
|
| Latest commit: |
e71b3ad
|
| Status: | ✅ Deploy successful! |
| Preview URL: | https://c23811a9.vald.pages.dev |
| Branch Preview URL: | https://backport-main-release-v1-7.vald.pages.dev |
kpango
changed the title
kpango
force-pushed
the
backport/main/release/v1.7
branch
from
3845156 to
eda634d
Compare
…dkit-syft-scanner reference to ghcr.io Signed-off-by: kpango <[email protected]>
kpango
force-pushed
the
backport/main/release/v1.7
branch
from
eda634d to
e71b3ad
Compare
![github-actions[bot]](https://avatars.githubusercontent.com/in/15368?s=60&v=4){kind=small}
| @@ -147,4 +142,4 @@ RUN --mount=type=bind,target=.,rw \ | |||
| && make faiss/install \ | |||
| && rm -rf ${GOPATH}/src/github.com/${ORG}/${REPO}/* | |||
| # skipcq: DOK-DL3002 | |||
| USER root:root | |||
| USER root:root | |||
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 0
Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Files ignored due to path filters (2)
go.sumis excluded by!**/*.sumrust/Cargo.lockis excluded by!**/*.lock
Files selected for processing (36)
- .gitfiles (12 hunks)
- .github/workflows/_docker-image.yaml (2 hunks)
- Makefile (2 hunks)
- Makefile.d/docker.mk (2 hunks)
- dockers/agent/core/agent/Dockerfile (3 hunks)
- dockers/agent/core/faiss/Dockerfile (3 hunks)
- dockers/agent/core/ngt/Dockerfile (3 hunks)
- dockers/agent/sidecar/Dockerfile (3 hunks)
- dockers/binfmt/Dockerfile (1 hunks)
- dockers/buildbase/Dockerfile (1 hunks)
- dockers/buildkit/Dockerfile (1 hunks)
- dockers/buildkit/syft/scanner/Dockerfile (1 hunks)
- dockers/ci/base/Dockerfile (3 hunks)
- dockers/dev/Dockerfile (3 hunks)
- dockers/discoverer/k8s/Dockerfile (3 hunks)
- dockers/gateway/filter/Dockerfile (3 hunks)
- dockers/gateway/lb/Dockerfile (3 hunks)
- dockers/gateway/mirror/Dockerfile (3 hunks)
- dockers/index/job/correction/Dockerfile (3 hunks)
- dockers/index/job/creation/Dockerfile (3 hunks)
- dockers/index/job/readreplica/rotate/Dockerfile (3 hunks)
- dockers/index/job/save/Dockerfile (3 hunks)
- dockers/index/operator/Dockerfile (3 hunks)
- dockers/manager/index/Dockerfile (3 hunks)
- dockers/operator/helm/Dockerfile (3 hunks)
- dockers/tools/benchmark/job/Dockerfile (3 hunks)
- dockers/tools/benchmark/operator/Dockerfile (3 hunks)
- dockers/tools/cli/loadtest/Dockerfile (3 hunks)
- go.mod (4 hunks)
- hack/docker/gen/main.go (9 hunks)
- k8s/discoverer/deployment.yaml (1 hunks)
- k8s/gateway/gateway/lb/deployment.yaml (1 hunks)
- k8s/index/operator/configmap.yaml (1 hunks)
- k8s/index/operator/deployment.yaml (1 hunks)
- k8s/manager/index/deployment.yaml (1 hunks)
- versions/JAEGER_OPERATOR_VERSION (1 hunks)
Files skipped from review due to trivial changes (25)
- dockers/agent/core/agent/Dockerfile
- dockers/agent/core/faiss/Dockerfile
- dockers/agent/core/ngt/Dockerfile
- dockers/agent/sidecar/Dockerfile
- dockers/binfmt/Dockerfile
- dockers/buildbase/Dockerfile
- dockers/buildkit/Dockerfile
- dockers/buildkit/syft/scanner/Dockerfile
- dockers/ci/base/Dockerfile
- dockers/dev/Dockerfile
- dockers/discoverer/k8s/Dockerfile
- dockers/gateway/filter/Dockerfile
- dockers/gateway/lb/Dockerfile
- dockers/gateway/mirror/Dockerfile
- dockers/index/job/correction/Dockerfile
- dockers/index/job/creation/Dockerfile
- dockers/index/job/readreplica/rotate/Dockerfile
- dockers/index/job/save/Dockerfile
- dockers/index/operator/Dockerfile
- dockers/manager/index/Dockerfile
- dockers/operator/helm/Dockerfile
- dockers/tools/benchmark/job/Dockerfile
- dockers/tools/benchmark/operator/Dockerfile
- dockers/tools/cli/loadtest/Dockerfile
- versions/JAEGER_OPERATOR_VERSION
Additional comments not posted (24)
.github/workflows/_docker-image.yaml (2)
51-53: Enhancement: Schedule trigger added.The addition of the schedule trigger expands the workflow's execution scenarios, which is beneficial for automated tasks.
84-84: Modification: Snapshotter flag updated.The addition of
--oci-worker-snapshotter=stargzmay enhance performance by using a different snapshotter. Ensure that this aligns with the project's goals and verify its impact.k8s/index/operator/deployment.yaml (1)
49-49: Update: ConfigMap checksum modified.The
checksum/configmapannotation has been updated, ensuring that the deployment references the latest configuration. This is crucial for maintaining consistency and correctness.k8s/discoverer/deployment.yaml (1)
49-49: Update: ConfigMap checksum modified.The
checksum/configmapannotation has been updated, ensuring that the deployment references the latest configuration. This is crucial for maintaining consistency and correctness.k8s/manager/index/deployment.yaml (1)
49-49: Checksum update for configmap is valid.The updated checksum value for the
configmapannotation is a standard practice to ensure the deployment uses the latest configuration.k8s/gateway/gateway/lb/deployment.yaml (1)
48-48: Checksum update for configmap is valid.The updated checksum value for the
configmapannotation ensures the deployment uses the latest configuration, which is a standard practice.Makefile.d/docker.mk (2)
71-71: Use of variable for Docker image reference improves maintainability.The introduction of
$(DEFAULT_BUILDKIT_SYFT_SCANNER_IMAGE)enhances flexibility and maintainability by centralizing the image reference, making future updates easier.
232-232: Variable assignment for Docker image reference is appropriate.Assigning
DEFAULT_BUILDKIT_SYFT_SCANNER_IMAGEensures consistent usage across the build process, aligning with best practices for maintainability.hack/docker/gen/main.go (4)
Line range hint
59-73: LGTM! Enhanced flexibility in Dockerfile generation.The introduction of conditional logic based on
AliasImageand the use ofBuildStageNameimprove the configurability of the Docker build process.
Line range hint
191-200: LGTM! Struct enhancements improve Docker configuration control.The addition of
AliasImageandBuildStageNamefields to theDatastruct allows for more granular control over Docker build stages and images.
234-234: LGTM! Default build stage name adds robustness.Setting
defaultBuildStageNameto "builder" ensures consistency in Docker build stages.
668-695: LGTM! Improved Docker configurations for specific applications.The configurations for
vald-buildbase,vald-buildkit,vald-binfmt, andvald-buildkit-syft-scannerenhance Docker setup and align with updated container references.k8s/index/operator/configmap.yaml (2)
Line range hint
17-17: LGTM! Version label updated to v1.7.13.The update from
v1.7.12tov1.7.13reflects the new release version and aligns with the PR objectives.
28-28: LGTM! Enhanced socket options for health checks.Setting
tcp_fast_openandtcp_no_delaytotruefor thereadinessserver likely improves performance by reducing latency.Makefile (3)
27-29: LGTM! Consistent naming for agent images.The use of
AGENT_IMAGEas a base for image names enhances clarity and maintainability.
35-35: LGTM! Consistent naming for Buildkit Syft Scanner image.The use of
BUILDKIT_IMAGEas a base forBUILDKIT_SYFT_SCANNER_IMAGEaligns with consistent naming conventions.
52-52: LGTM! Default image tag for Buildkit Syft Scanner.The introduction of
DEFAULT_BUILDKIT_SYFT_SCANNER_IMAGEprovides a clear default, enhancing clarity in image configuration.go.mod (5)
228-228: Verify test compatibility withginkgoupdate.The update from
v2.19.1tov2.20.0might include new testing features or changes. Ensure that all test cases are compatible with this version.
25-25: Verify compatibility and test coverage forazcoreupdate.The update from
v1.13.0tov1.14.0might introduce new features or changes. Ensure that the codebase is compatible with this version and that any new features are covered by tests.
317-317: Verify API compatibility withgoogle.golang.org/apiupdate.The update to
v0.191.0might include new APIs or changes. Ensure that the codebase is compatible with this version and that any new APIs are correctly integrated.
400-400: Verify compatibility withgolang.org/x/oauth2update.The update to
v0.22.0might include improvements or changes. Ensure that the codebase is compatible with this version and that any new features are utilized if applicable.
404-404: Verify compatibility withgolang.org/x/timeupdate.The update to
v0.6.0might include improvements or changes. Ensure that the codebase is compatible with this version and that any new features are utilized if applicable..gitfiles (2)
88-88: Addition of new workflow file.The addition of
.github/workflows/dockers-buildkit-syft-scanner-image.yamlindicates a new workflow related to Docker image scanning, aligning with the PR objectives.
528-528: Addition of new Dockerfile.The addition of
dockers/buildkit/syft/scanner/Dockerfilesuggests a new or updated Docker image configuration for the syft scanner, consistent with the PR's objectives.
There was a problem hiding this comment.
Actionable comments posted: 0
Outside diff range, codebase verification and nitpick comments (2)
k8s/index/operator/configmap.yaml (1)
Inconsistent Version Update Detected
The version update from
v1.7.12tov1.7.13is not fully consistent across the repository. The following file still contains the old version and needs to be updated:
cmd/index/operator/sample.yamlPlease update this file to ensure consistency in versioning across all components.
Analysis chain
Line range hint
11-13:
Ensure Consistency in Version Updates.The version updates for
app.kubernetes.io/versionandhelm.sh/chartfromv1.7.12tov1.7.13are consistent. Ensure that all related components and documentation reflect this version change.Also applies to: 17-19
Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Description: Verify consistency of version updates across the repository. # Test: Search for version numbers in YAML files. Expect: Only occurrences of the new version. rg --type yaml 'v1\.7\.13'Length of output: 31904
Script:
#!/bin/bash # Description: Check for any remaining instances of the old version in YAML files. # Test: Search for old version numbers in YAML files. Expect: No occurrences of the old version. rg --type yaml 'v1\.7\.12'Length of output: 1220
Makefile (1)
35-35: Ensure Consistent Use ofDEFAULT_BUILDKIT_SYFT_SCANNER_IMAGEThe variable
DEFAULT_BUILDKIT_SYFT_SCANNER_IMAGEis not used consistently across the build configuration. InMakefile.d/docker.mk, there are instances whereBUILDKIT_SYFT_SCANNER_IMAGEis used directly without theDEFAULT_prefix. To enhance clarity and maintainability, ensure thatDEFAULT_BUILDKIT_SYFT_SCANNER_IMAGEis used consistently wherever the default image is intended.
- Makefile.d/docker.mk: Replace direct references to
BUILDKIT_SYFT_SCANNER_IMAGEwithDEFAULT_BUILDKIT_SYFT_SCANNER_IMAGEwhere applicable.Analysis chain
Enhance Clarity with Default Image Variable.
The introduction of
DEFAULT_BUILDKIT_SYFT_SCANNER_IMAGEclarifies the build process for the BuildKit Syft scanner by explicitly defining a default image. Ensure this variable is used consistently across relevant scripts and configurations.Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Description: Verify the usage of `DEFAULT_BUILDKIT_SYFT_SCANNER_IMAGE`. # Test: Search for the new variable in Makefiles and scripts. Expect: Consistent usage across the build process. rg 'DEFAULT_BUILDKIT_SYFT_SCANNER_IMAGE'Length of output: 327
Script:
#!/bin/bash # Search for all instances of BUILDKIT_SYFT_SCANNER_IMAGE to ensure consistent usage of DEFAULT_BUILDKIT_SYFT_SCANNER_IMAGE rg 'BUILDKIT_SYFT_SCANNER_IMAGE'Length of output: 523
Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Files ignored due to path filters (2)
go.sumis excluded by!**/*.sumrust/Cargo.lockis excluded by!**/*.lock
Files selected for processing (36)
- .gitfiles (12 hunks)
- .github/workflows/_docker-image.yaml (2 hunks)
- Makefile (2 hunks)
- Makefile.d/docker.mk (2 hunks)
- dockers/agent/core/agent/Dockerfile (3 hunks)
- dockers/agent/core/faiss/Dockerfile (3 hunks)
- dockers/agent/core/ngt/Dockerfile (3 hunks)
- dockers/agent/sidecar/Dockerfile (3 hunks)
- dockers/binfmt/Dockerfile (1 hunks)
- dockers/buildbase/Dockerfile (1 hunks)
- dockers/buildkit/Dockerfile (1 hunks)
- dockers/buildkit/syft/scanner/Dockerfile (1 hunks)
- dockers/ci/base/Dockerfile (3 hunks)
- dockers/dev/Dockerfile (3 hunks)
- dockers/discoverer/k8s/Dockerfile (3 hunks)
- dockers/gateway/filter/Dockerfile (3 hunks)
- dockers/gateway/lb/Dockerfile (3 hunks)
- dockers/gateway/mirror/Dockerfile (3 hunks)
- dockers/index/job/correction/Dockerfile (3 hunks)
- dockers/index/job/creation/Dockerfile (3 hunks)
- dockers/index/job/readreplica/rotate/Dockerfile (3 hunks)
- dockers/index/job/save/Dockerfile (3 hunks)
- dockers/index/operator/Dockerfile (3 hunks)
- dockers/manager/index/Dockerfile (3 hunks)
- dockers/operator/helm/Dockerfile (3 hunks)
- dockers/tools/benchmark/job/Dockerfile (3 hunks)
- dockers/tools/benchmark/operator/Dockerfile (3 hunks)
- dockers/tools/cli/loadtest/Dockerfile (3 hunks)
- go.mod (4 hunks)
- hack/docker/gen/main.go (9 hunks)
- k8s/discoverer/deployment.yaml (1 hunks)
- k8s/gateway/gateway/lb/deployment.yaml (1 hunks)
- k8s/index/operator/configmap.yaml (1 hunks)
- k8s/index/operator/deployment.yaml (1 hunks)
- k8s/manager/index/deployment.yaml (1 hunks)
- versions/JAEGER_OPERATOR_VERSION (1 hunks)
Files skipped from review due to trivial changes (27)
- .gitfiles
- dockers/agent/core/agent/Dockerfile
- dockers/agent/core/faiss/Dockerfile
- dockers/agent/core/ngt/Dockerfile
- dockers/agent/sidecar/Dockerfile
- dockers/binfmt/Dockerfile
- dockers/buildbase/Dockerfile
- dockers/buildkit/Dockerfile
- dockers/buildkit/syft/scanner/Dockerfile
- dockers/ci/base/Dockerfile
- dockers/dev/Dockerfile
- dockers/discoverer/k8s/Dockerfile
- dockers/gateway/filter/Dockerfile
- dockers/gateway/lb/Dockerfile
- dockers/gateway/mirror/Dockerfile
- dockers/index/job/correction/Dockerfile
- dockers/index/job/creation/Dockerfile
- dockers/index/job/readreplica/rotate/Dockerfile
- dockers/index/job/save/Dockerfile
- dockers/index/operator/Dockerfile
- dockers/manager/index/Dockerfile
- dockers/operator/helm/Dockerfile
- dockers/tools/benchmark/job/Dockerfile
- dockers/tools/benchmark/operator/Dockerfile
- dockers/tools/cli/loadtest/Dockerfile
- k8s/manager/index/deployment.yaml
- versions/JAEGER_OPERATOR_VERSION
Additional comments not posted (15)
.github/workflows/_docker-image.yaml (2)
51-53: LGTM! The addition of the schedule trigger is appropriate.This change enhances the flexibility of the workflow by allowing it to run on scheduled events.
84-84: LGTM! The update to buildkitd-flags is appropriate.Including
--oci-worker-snapshotter=stargzcan optimize the image building process.k8s/index/operator/deployment.yaml (1)
49-49: LGTM! The config map checksum update is necessary.Updating the checksum ensures that the deployment uses the latest configuration.
k8s/discoverer/deployment.yaml (1)
49-49: LGTM! The config map checksum update is necessary.Updating the checksum ensures that the deployment uses the latest configuration.
k8s/gateway/gateway/lb/deployment.yaml (1)
48-48: Checksum update approved.The
checksum/configmapannotation has been updated to reflect changes in the configmap. This ensures that the deployment uses the latest configuration.Makefile.d/docker.mk (1)
71-71: Enhancement approved for buildkit-syft-scanner image configurability.The use of
$(DEFAULT_BUILDKIT_SYFT_SCANNER_IMAGE)improves flexibility and maintainability of the Docker build process.Also applies to: 232-232
hack/docker/gen/main.go (3)
57-61: Dynamic build stage name logic approved.The introduction of
AliasImageandBuildStageNameenhances flexibility in Dockerfile generation, allowing for more dynamic and customizable builds.Also applies to: 72-73, 129-130, 144-144
668-695: New application configurations approved.The additions for
vald-buildbase,vald-buildkit,vald-binfmt, andvald-buildkit-syft-scannerimprove the Dockerfile generation process by providing specific configurations for these applications. This enhances the build system's organization and clarity.
Line range hint
191-200: Verify usage of new struct fields.The new fields
AliasImageandBuildStageNamein theDatastruct are crucial for the updated Dockerfile generation logic. Ensure these fields are correctly utilized throughout the codebase.Also applies to: 234-234, 725-727
Verification successful
Verified usage of new struct fields.
The fields
AliasImageandBuildStageNameare utilized in thehack/docker/gen/main.gofile. They are integrated into the Dockerfile generation logic, withAliasImageused in conditional logic andBuildStageNamein template expressions. This confirms their usage as intended.
AliasImageis used in conditional template logic.BuildStageNameis used in Dockerfile generation and assigned default values.Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Description: Verify usage of new fields in the `Data` struct. # Test: Search for the usage of `AliasImage` and `BuildStageName`. # Expect: Occurrences where these fields are used in logic or templates. rg --type go 'AliasImage|BuildStageName'Length of output: 905
k8s/index/operator/configmap.yaml (1)
28-28: Review Socket Option Changes.The
tcp_corkoption formetrics_servershas been changed fromfalsetotrue. This may affect network performance by delaying packet transmission until a full packet is formed. Ensure this change aligns with performance objectives.Makefile (1)
27-29: Standardize Image Naming Conventions.The changes to
AGENT_NGT_IMAGE,AGENT_FAISS_IMAGE, andAGENT_SIDECAR_IMAGEimprove consistency by using the$(AGENT_IMAGE)variable. This enhances maintainability and reduces errors.go.mod (4)
400-400: Check OAuth2 and Time Library Updates.The updates to
golang.org/x/oauth2andgolang.org/x/timemay affect functionality. Ensure compatibility with existing code and verify no breaking changes.Also applies to: 404-404
317-317: Ensure Google API Update Compatibility.The update of
google.golang.org/apifromv0.190.0tov0.191.0may introduce changes. Verify compatibility and ensure no breaking changes affect API interactions.
228-228: Check Ginkgo Dependency Update.The update of
github.com/onsi/ginkgo/v2fromv2.19.1tov2.20.0may affect test behavior. Ensure all tests are compatible with the new version.Verification successful
No Ginkgo Usage Detected in Codebase
The update to
github.com/onsi/ginkgo/v2fromv2.19.1tov2.20.0does not appear to affect the codebase, as no usage of Ginkgo was detected in any code files. The dependency is present in configuration files but not actively used in the code.Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Description: Verify compatibility of Ginkgo update. # Test: Search for Ginkgo usage in test files. Expect: No issues with the updated version. ast-grep --lang go --pattern 'ginkgo.$_'Length of output: 42
Script:
#!/bin/bash # Description: Find files that import Ginkgo and examine the context of its usage. # Search for import statements of Ginkgo in Go files rg --glob '*.go' 'import\s+\(?\s*"github.com/onsi/ginkgo/v2"' -A 5 # List all test files to check for Ginkgo usage fd --extension go --exec rg 'ginkgo' {}Length of output: 110
Script:
#!/bin/bash # Description: Search for any references to Ginkgo in the repository. # Search for any occurrence of 'ginkgo' in all files, including comments and documentation. rg 'ginkgo' -iLength of output: 676
25-25: Verify Compatibility of Azure SDK Update.The update of
github.com/Azure/azure-sdk-for-go/sdk/azcorefromv1.13.0tov1.14.0may introduce changes. Verify compatibility with existing code and ensure no breaking changes affect functionality.
Description
SSIA
Related Issue
Versions
Checklist
Special notes for your reviewer
Summary by CodeRabbit
New Features
dockers/buildkit/syft/scanner, improving the build infrastructure.Improvements
Bug Fixes
go.modfile to ensure compatibility and leverage improvements.Chores